Thank you for your contribution, it is very much appreciated. I will try to review it in the next few days, as there's already another fairly big PR currently in the works.

Rebased

@virzak The sync-method-generator library is really impressive. It greatly reduces the need to write duplicate code by leveraging the power of SourceGenerator technology — I like it and have starred your project~

However, the author of this project may be hesitant to introduce third-party dependencies. Would it be feasible to extract and adapt a subset of your library's core functionalities, and integrate them into a new project — perhaps named MiniExcelLibs.Analyzer?

@michelebastione @shps951023 What are your thoughts on introducing third-party dependencies and using SourceGenerator technology to automatically generate synchronous methods, so that we only need to write asynchronous methods? There might be a slight risk with the SourceGenerator.

@izanhzh, thank you for the ⭐ and the kind words. Yes, extract whatever you want, it is open source. Crediting and linking the original project in README would be appreciated, if you went that route.

This library was made as safe as possible to for integration as a third party as well:

• Test Driven Development at all times with all warnings treated as errors
• 95% code coverage
• Fast response to issues
• I personally used it in client project
• It is used in other large projects
  • FluentValidation
  • There is a case of 3K generated methods in a single project.
• It has my full commitment

Also, integrating this library with MiniExcel uncovered some compilation errors in earlier versions of the generator, and the generator had to be patched.

@virzak @izanhzh I think this project is really neat, congrats on being a dependency for FluentValidation!
But I also believe there's value in having a complete dependency free library if possible, so the preferred thing to do would be to integrate the required code.

That said, and please note this is just my humble opinion, I think source generators in a library should be used only when the code to generate has to reference the consuming applications's assemblies. This would not be the case for this addition. I might be wrong but I believe all the functionalities of a library should be known before it is compiled. But again, this is just my personal take, @shps951023 please let us know your stance.

I would also leave in the old async implementation for backwards compatibility and call the new methods something like MiniExcel.EnumerateAsync, same thing with dropping the net45 support in favor of .net462, it's probably gonna affect almost nobody, but still I don't think we should do that yet.

I hope I don't sound overly critical, I'm just expresing my concerns as this are all very fundamental changes.
Now that the weekend has finally arrived I will open a separate PR with my implementation as soon as I can (I swear my code exists, I'm not making it up 😅) so that we can decide how to move forwards and maybe merge the two versions if that's viable.

dependency for FluentValidation

Thanks, @michelebastione! Took a while to convince the maintainer lol.

complete dependency free library if possible

Now that I gave it more thought, I realized this might be a harder task than it seems.
Sync method generator relies on other source generators / libraries / source only helpers:

• IsExternalInit
• N.SourceGenerators.UnionTypes
• PolySharp

You'd need to "flatten" all of these in order to even compile sync method generator. Possible, but not sure it is worth it.

Also consider what could happen if I believed in this approach and actually flattened UnionTypes library in sync method generator back in 2023. It could diverge from upstream by today and MiniExcel would have to depend on my version of UnionTypes. Not sure if this won't end up being a burden in the long run.

library should be known before it is compiled

Any time you forgo source generation, you are introducing code duplication. In my opinion (and I'm obviously biased), there is nothing more dangerous in software development that code duplication, which would introduce more work and will be error prone. One solution to "assert control" is to extract public method into an interfaces, which won't be source generated. That way the signature (i.e. the only thing that users are really concerned about) is not source generated.

leave in the old async implementation for backwards compatibility and call the new methods something like MiniExcel.EnumerateAsync

This is another trade-off. Avoiding breaking change vs following conventions + less methods.

Without knowing what is the base of users and how many of them are using the old signatures, I'd say that breaking changes are happening all the time. It usually takes a few minutes to update, so I'd remove it, since I'm unsure how Task<IEnuemerable> could be preferred over IAE or Task<IList>. But there are other options that I like as well. Rather than having:

IAsyncEnumerable<T> EnumerateAsync()
Task<IEnumerable<T>> QueryAsync()
IEnumerable<T> Query()

You can have:

IAsyncEnumerable<T> EnumerateAsync()
Task<IEnumerable<T>> QueryAsync()
IEnumerable<T> Enumerate()
[Obsolete]
IEnumerable<T> Query() //=> redirects to Enumerate

or

IAsyncEnumerable<T> QueryAsync()
Task<IEnumerable<T>> QueryIEnumerableAsync()
IEnumerable<T> Query()

That way at least the sync and async versions are named the same, which is the convention and won't be a surprise to new users of the library.

dropping the net45 support in favor of .net462

I tried to leave net45 in place, but was running into compilation issues - so I switched to the latest supported version. I think there is as much merit to support that version as there is to support .NET 7, which this project doesn't support.

I will open a separate PR with my implementation

Looking forward to seeing it. There is a single (ReadBigExcel_TakeCancel_Throws_TaskCanceledException) test case which passes only 50% of the time. Also looking to see how you tackled other issues, I had struggles with.

All tests are now passing, ready for review.

Before we merge there are still a couple of points I would like to address, nothing major but I believe it's worth talking about it. I'll be writing them down shortly if you have the patience to bear with me.

@michelebastione To be honest, if the SyncGenerator can't support net45, removing net45 support would be considered a significant breaking change. This is definitely a tough decision to make. I'm not sure how many people are still using net45, but maintaining both synchronous and asynchronous methods side by side can be quite cumbersome. If we decide to adopt SyncGenerator, this would definitely be a bold attempt. While SyncGenerator provides some convenience, it may also introduce challenges in code maintenance. For example, the generated code might not handle certain special async method calls well. I'm not entirely familiar with the underlying mechanics.

I don't have a strong recommendation at this point. Let's see what the @shps951023 has to say about this.

@izanhzh, Sync Method Generator supports net45. However net45 does not support IAsyncEnumerable - at least I was unable to get it to work.

I think that would be a mess of preprocessor instructions and we would still have to define the sync methods from scratch, which goes against the point of including the dependency in the first place.
I must stay, despite being skeptical in the beginning (and even a little bit now) I am liking how the final result came out to be, it's simpler, more concise and cleaner.
Maybe this could be a good opportunity to start a MiniExcel 2.0, we could release it in beta for a few versions to test the stability and also implement other possibly breaking changes like bumping the LangVersion and moving the CSV logic in a separate assembly.
And we could offer limited support for some time for people still on net45 who'd have to keep using version 1.x.

@michelebastione, thanks for approving!

Do I get to Squash and merge it or will someone else do it?

@virzak Given the sheer amount of change this PR brings, I think @shps951023 should give the final word, especially in regards to making this the start of a new major version and subsequent separation of branches like I suggested in my previous comment.

Dear team,
Agree to upgrade version despondency.
Years ago, we need to maintain compatibility with .NET Framework 4.5 because many users (especially in manufacturing) were still using older systems that relied on MS framework.
However, nowadays we can drop support for 4.5 and move to higher versions, since very few people are still using the 4.5 framework. e.g. Dapper now only supports 4.6.1, .NET Standard 2.0, and .NET 8.0.

We can follow what @michelebastione suggested and upgrade to a new major version.
Like @izanhzh said, maintaining compatibility with the older async framework has become a significant burden for our team, so it makes sense to make this change now. 🙌🙌

@michelebastione Leaving squash merging / branching out to you. I'm unsure if this should go to master or a feature branch.

Alright, cool! I created a new branch v1.x-maintenance where we can keep the legacy version. Next steps after merging should be, in my opinion:

• Making a final v1.x release where we explain the direction we're taking with future versions of the library
• Decide on a deadline for v1 end of life
• Making a v2.0.0-beta.1 release so that we can start receiving feedback
• Planning subsequent beta releases incorporating feedback / refactoring / new features until we're ready for a stable v2 release

I'm writing down a brief commit message and merging shortly.

@virzak Please see the image below. There are a lot of SYNC_ONLY, which makes it very cluttered. Does your library support define some Map conversion mapping? We know that MiniExcelAsyncStreamWriter is the asynchronous class of MiniExcelStreamWriter. Is there any way to further optimize and avoid SYNC_ONLY?

@izanhzh Currently sync method generator only deals with methods in the same class. Perhaps we can improve this in the future.

@izanhzh Currently sync method generator only deals with methods in the same class. Perhaps we can improve this in the future.

I've created a new PR #807 that auto-generates more synchronous methods from asynchronous ones, reducing the use of SYNC_ONLY.